-
-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Android gradle plugin #633
Conversation
|
/LinuxIntermediate/Build/Linux/UnrealGame/Shipping/Sentry/* | ||
/LinuxIntermediate/Build/Linux/x64/UnrealGame/Development/Sentry/* | ||
/LinuxIntermediate/Build/Linux/x64/UnrealGame/Shipping/Sentry/* | ||
/Gradle/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that change intentional? The how do the LinuxIntermediate
related to Gradle
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, aside from adding Gradle
here I've also removed LinuxIntermediate
stuff which is now redundant (probably I've missed deleting these entries earlier). Should we tackle this kind of clean-up in a separate PR?
@@ -137,7 +141,7 @@ | |||
<buildscriptGradleAdditions> | |||
<insert> | |||
dependencies { | |||
classpath 'com.android.tools.build:gradle:3.5.4' | |||
classpath 'com.android.tools.build:gradle:7.4.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the gradle tool itself, right? And that's a requirement for the sentry-android-gradle-plugin 4.11.0
?
Are we going to mess with user's build by messing with the gradle version that comes with the UE installation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the io.sentry.android.gradle >= 3.0.0
requires Android Gradle Plugin >= 7.0.0
according to docs.
I've tested this with a few different engine versions (including pretty old UE 4.27) and the build was successful however there's always a chance that some compatibility issues with 3rd-party plugins can arise.
UPD: Added new plugin setting which allows switching back to legacy Sentry Android Gradle plugin in case of any compatibility issues with other third-party plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I see how this fixes the issue linked. The issue was resolved by adding the defaults url
, right? How does the newer Android Gradle Plugin help with this? Is there anything else we're getting out of it?
The fallback is great, I really appreciate it, changing the user's Gradle version still makes me nervous.
</insert> | ||
</true> | ||
<false> | ||
<log text="Using Sentry Gradle plugin 4.11.0 for Android debug symbol upload."/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the hardcoded version number going to bite us in the butt later on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't have any means allowing to check how Sentry Gradle plugin performs in our CI probably it'd be safer to keep a hardcoded version here instead of bumping it automatically for the time being.
With the newer Gradle plugin version adding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that we cannot offer ootb support for self-hosted and there is a fallback/opt-out for those that need it: LGTM! 🔥
This PR is supposed to address Android symbol upload issues that some Sentry self-hosted users faced previously due to the legacy gradle plugin being in use (i.e. #502).
In order to bump the Android gradle plugin version it was necessary to override engine's gradle version as well by replacing
gradle-wrapper.properties
file in build dir.